Skip to content

Replace Python tests with GO test #45503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

khushijain21
Copy link
Contributor

@khushijain21 khushijain21 commented Jul 22, 2025

Proposed commit message

This PR moves Python tests to GO

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 22, 2025
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

mergify bot commented Jul 22, 2025

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @khushijain21? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@khushijain21 khushijain21 changed the title Remove TestCmd Python Test Replace Python tests with GO test Jul 22, 2025
@khushijain21 khushijain21 marked this pull request as ready for review July 23, 2025 15:48
@khushijain21 khushijain21 requested a review from a team as a code owner July 23, 2025 15:48
@khushijain21 khushijain21 requested review from mauri870, faec and rdner July 23, 2025 15:48
@khushijain21 khushijain21 changed the title Replace Python tests with GO test [WIP] Replace Python tests with GO test Jul 23, 2025
@rdner
Copy link
Member

rdner commented Jul 24, 2025

We should rename and split filebeat/testing/integration/sample_test.go into meaningful test files. e.g. cmd_test.go, cmd_modules_test.go, etc. It should not all be in one giant file.

@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jul 25, 2025
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 25, 2025

def test_registrar_file_content(self):
"""
Check if registrar file is created correctly and content is as expected
Copy link
Contributor Author

@khushijain21 khushijain21 Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khushijain21 khushijain21 changed the title [WIP] Replace Python tests with GO test Replace Python tests with GO test Jul 25, 2025
@khushijain21 khushijain21 requested a review from rdner July 25, 2025 15:56
Copy link
Member

@mauri870 mauri870 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few minor comments. Thanks for your work on this and for cleaning up the tests!

@khushijain21 khushijain21 requested a review from rdner July 31, 2025 06:00
Copy link
Member

@rdner rdner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see the framework evolving as we see more test-cases.

There are a few minor comments. I would approve after the got addressed.


config := `
filebeat.inputs:
- type: log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd encourage to test the filestream input only because log is deprecated.

Also, there is known issue with --once #33718


config := `
filebeat.inputs:
- type: container
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's test filestream with the container parser instead as described here https://www.elastic.co/docs/reference/beats/filebeat/filebeat-input-container.

The container input is now deprecated.

}
}

type registryEntry struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything registry should be in a separate file and be available to all the tests as a set of helper functions.


config := `
filebeat.inputs:
- input_type: log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be another test for the container input as well. Expect the same behavior.

}

// counts number of lines in the given file and asserts if it matches expected count
func CountLinesInFile(t *testing.T, path string, count int) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be near the integration.GenerateLogFile function as a part of the framework.

Comment on lines +92 to +100
InputDoc := mapstr.M{
"message": "sample dummy message",
"fields": mapstr.M{
"number": 20,
"slice": []string{"20", "30", "40"},
},
"isthisboolean": false,
"nilValue": nil,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to have raw JSON here, then parse it to mapstr.M and test the whole cycle. So, we're as close to the real use-case as possible.

}

// GetTempDir implements the BeatTest interface.
func (b *beatTest) GetTempDir() string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants